Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[CI] Added workflow for publishing the Solr image #303

Merged
merged 1 commit into from
Dec 7, 2023
Merged

[CI] Added workflow for publishing the Solr image #303

merged 1 commit into from
Dec 7, 2023

Conversation

mnocon
Copy link
Contributor

@mnocon mnocon commented Dec 6, 2023 •
edited
Loading

A little backstory (the way I remember it):

  1. The solr image orginally has been added in Product Catalog - https://github.com/ibexa/product-catalog/pull/24
  2. Because we needed it in public repositories as well the package was moved (without rebuilding it - it was republished) from product-catalog to core (https://github.com/ibexa/product-catalog/pull/479)
  3. This process was not automated and I don't think it's possible to trace the original Dockerfile that was used to create the image (there's one in the first linked PR and another in IBX-522: Introduced Docker image for Solr standalone ezsystems/ezplatform-solr-search-engine#214)

I wrote a little Dockerfile that:

  1. Generates the Solr config using the script for ibexa/solr package - this way we don't have to do any in-place editing of the config
  2. Combines the Solr config with the default Solr image

The default Solr config from the Solr image contains some settings that we do not want - for example the managed schema is enabled. To get rid of it I'm deleting everything first - the only configuration that will be used will the one that we provide.

This is combined with a simple Github Actions workflow that can be used to publish the image - so that we won't have to go through this again 🙈

Not done: versioning. The Solr configuration is always taken from master - and the image is always tagged as latest. We probably will have to change this in the future, but I don't have a clear way how it should look - and I'd like to make the CI green in this repository before the beta tag.

Contains TMP commits that need to be removed:

I have no idea what happened to PHPStan, maybe new version has been released?

@mnocon mnocon marked this pull request as ready for review December 7, 2023 00:34
@adamwojs adamwojs requested review from ibexa-yuna and a team December 7, 2023 07:35
@mnocon mnocon requested a review from Steveb-p December 7, 2023 08:28
ibexa-yuna
ibexa-yuna reviewed Dec 7, 2023
View reviewed changes
Copy link
Contributor

@ibexa-yuna ibexa-yuna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, however, I would like to provide a suggestion:

  1. Make sure that the image is rebuild & tested
  2. If tests are a success then push it
  3. Make the image build a monthly job

@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Dec 7, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@mnocon
Copy link
Contributor Author

mnocon commented Dec 7, 2023

LGTM, however, I would like to provide a suggestion:

1. Make sure that the image is rebuild & tested

2. If tests are a success then push it

3. Make the image build a monthly job

Thanks, those are good suggestions! I will include them in a follow-up to this (coming soon ™️ )

@adamwojs adamwojs merged commit 417c480 into ibexa:main Dec 7, 2023
12 of 15 checks passed
alongosz
alongosz reviewed Dec 7, 2023
View reviewed changes
.github/workflows/solr.yaml
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

naming: let's have here more specific name: gha-docker-solr.yaml, solr can lead to many expectations, while the purpose of this is to create image for gha-run tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I will include this note in a follow-up as well

alongosz
alongosz reviewed Dec 7, 2023
View reviewed changes
docker/solr/Dockerfile
RUN rm -rf server/solr/configsets/_default/conf/*
USER solr
COPY --from=builder /home/user/config server/solr/configsets/_default/conf
CMD ["solr-precreate", "collection1"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure where this comment ended up, but my question here was if we're able to run multicore tests somehow. collection1 is dedicated to single core tests, while we have several jobs (do we still in core?) testing also multi-core behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The multicore tests are run in the solr package. I believe it should be possible to include them in this repository - and in the Solr image - we would have create different cores.

I wouldn't do this now, can we come back to this after the release?

@mnocon mnocon mentioned this pull request Dec 12, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ibexa-yuna ibexa-yuna ibexa-yuna left review comments

@alongosz alongosz alongosz left review comments

@glye glye glye approved these changes

@adamwojs adamwojs adamwojs approved these changes

@Steveb-p Steveb-p Awaiting requested review from Steveb-p

Assignees

@kisztof kisztof

@mikadamczyk mikadamczyk

@alongosz alongosz

Labels
Ready for review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@mnocon @adamwojs @glye @alongosz @ibexa-yuna @kisztof @mikadamczyk